Skip to content

fix(core): prefer pwsh.exe over Windows PowerShell 5.1 (#25859)#25900

Merged
scidomino merged 9 commits into
google-gemini:mainfrom
kaluchi:fix/windows-prefer-pwsh-25859
May 18, 2026
Merged

fix(core): prefer pwsh.exe over Windows PowerShell 5.1 (#25859)#25900
scidomino merged 9 commits into
google-gemini:mainfrom
kaluchi:fix/windows-prefer-pwsh-25859

Conversation

@kaluchi

@kaluchi kaluchi commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #25859. Related: closes-as-effectively-same #18374; addresses the feature-request spirit of #15493, #2353, #6413.

On Windows, run_shell_command with embedded double quotes (e.g. node -e 'console.log(\"test\")') fails because Windows PowerShell 5.1 silently strips \" during native-command argument passing. PowerShell 7 (pwsh.exe) uses standards-compliant argument passing (\$PSNativeCommandArgumentPassing = 'Windows' by default since 7.3) and does not exhibit this behavior.

This PR makes getShellConfiguration prefer pwsh.exe from PATH over powershell.exe on Windows. Users with PowerShell 7 installed get the fix; users without keep the existing behavior — no regression either way.

Context

Windows PowerShell 5.1 is in legacy maintenance mode. Microsoft itself displays this banner when 5.1 starts:

```
Windows PowerShell
Copyright (C) Microsoft Corporation. All rights reserved.

Install the latest PowerShell for new features and improvements!
https://aka.ms/PSWindows
```

PS 5.1 is .NET Framework-bound, Windows-only, and lags PS 7 on shell behavior. `$PSNativeCommandArgumentPassing` (fixes native-cmd arg passing) and bash-style `&&`/`||` chain operators were added in PS 7 and cannot be backported to 5.1.

Respecting the user's environment. gemini-cli is a developer tool, and developers who work on Windows overwhelmingly install PowerShell 7 — it's the modern, cross-platform, actively-maintained runtime Microsoft recommends. Currently gemini-cli ignores the user's choice and pins Windows invocations to `powershell.exe` (5.1 on any modern Windows) regardless of whether pwsh 7 is installed. This PR follows both Microsoft's own guidance and the user's clear intent: prefer PS 7 when it's on their system, fall back to 5.1 only when it isn't.

Side benefits

PowerShell 7 also addresses other Windows-specific pain reported against gemini-cli:

These aren't explicitly targeted by the fix but improve as a natural consequence for users who have pwsh 7 installed.

Verification

Reproduce directly in each shell — same commands, different results.

Issue 1: embedded double quotes stripped (#25859)

Windows PowerShell 5.1:
```
PS C:\Users\EugenPC> node -e 'console.log("test")'
[eval]:1
console.log(test)
^
ReferenceError: test is not defined
at [eval]:1:13
...
Node.js v22.18.0
```

PowerShell 7.6.1:
```
PS C:\Users\EugenPC> node -e 'console.log("test")'
test
```

Issue 2: `&&` / `||` operators rejected (#20773, #21997, #18022)

Windows PowerShell 5.1:
```
PS C:\Users\EugenPC> echo a && echo b
At line:1 char:8

  • echo a && echo b
  •    ~~
    

The token '&&' is not a valid statement separator in this version.
```

PowerShell 7.6.1:
```
PS C:\Users\EugenPC> echo a && echo b
a
b
```

With this PR, gemini-cli routes through `pwsh.exe` on Windows when it's installed, inheriting the correct behavior for both issues.

Behavior matrix

Environment Before After
Windows + `pwsh.exe` in PATH powershell pwsh (fix)
Windows, no `pwsh.exe` powershell powershell
Windows, explicit `ComSpec` honored honored
Windows sandbox (`windows-native`) cmd.exe /c cmd.exe /c
macOS / Linux bash bash

Only the first row changes behavior.

Tests

  • Unit tests in `shell-utils.test.ts` covering new pwsh preference, path-extension handling, unset `PATH`.
  • New Windows-only integration file (`shellExecutionService.windows.integration.test.ts`, skipped on non-Windows): 5 cases driving real spawns through `ShellExecutionService`, asserting byte-exact preservation of embedded `"`.
  • Full core suite: 0 regressions vs. `main`.

Root cause

PR #11157 (2025-10-16) changed the default Windows shell from `cmd.exe /d /s /c` to `powershell.exe -NoProfile -Command`. Windows PowerShell 5.1 lacks `$PSNativeCommandArgumentPassing` (introduced in PS 7.2 as experimental, default since 7.3) and has a long-standing runtime limitation: `"` inside arguments is dropped when invoking native executables. Fixable only by running through PS 7 — which is what this PR does.

@kaluchi kaluchi requested a review from a team as a code owner April 24, 2026 01:41
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request improves shell command execution on Windows by prioritizing PowerShell 7 (pwsh.exe) over the legacy Windows PowerShell 5.1. This change resolves long-standing issues with argument quoting and improves compatibility with modern shell features like '&&' and '||' operators, while maintaining backward compatibility for users without PowerShell 7 installed.

Highlights

  • PowerShell 7 Preference: Updated the shell configuration to prefer 'pwsh.exe' (PowerShell 7) over 'powershell.exe' (Windows PowerShell 5.1) on Windows when available in the PATH.
  • Resolved Argument Quoting Issue: Addressed the issue where Windows PowerShell 5.1 incorrectly strips embedded double quotes from arguments passed to native executables.
  • Synchronous Executable Resolution: Refactored 'resolveExecutable' to be synchronous, allowing it to be used during shell configuration initialization.
  • New Integration Tests: Added Windows-specific integration tests to verify that shell commands with inline double quotes are executed correctly.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-cli gemini-cli Bot added the priority/p2 Important but can be addressed in a future release. label Apr 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the shell configuration to prefer pwsh.exe on Windows to resolve quoting issues and adds integration tests for verification. It also refactors the resolveExecutable utility to be synchronous and updates the test suite to use vi.stubEnv. Feedback indicates that the switch to synchronous I/O blocks the event loop and violates project conventions, identifies a regression in Windows executable resolution for extensionless files, and suggests using asynchronous operations with real-path resolution for better security and performance.

Comment thread packages/core/src/utils/shell-utils.ts
Comment thread packages/core/src/utils/shell-utils.ts
Comment thread packages/core/src/utils/shell-utils.ts Outdated
Comment thread packages/core/src/utils/shell-utils.ts
@kaluchi

kaluchi commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@kaluchi

kaluchi commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author
pimg-20260424_203311 For reference — GitHub's own https://github.com/github/copilot-cli ships with pwsh as the default shell on Windows. Same repro from this PR (node -e 'console.log("test")') runs cleanly there, for the same reason this PR fixes it in gemini-cli.

@kaluchi

kaluchi commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Note this PR is non-breaking by design: Copilot CLI refuses to run without pwsh 7 (#847) and won't support 5.1 (#411, not planned). Here, 5.1 users keep working exactly as before — only users who already installed pwsh get the fix.

@kaluchi kaluchi force-pushed the fix/windows-prefer-pwsh-25859 branch from 506111a to 650765e Compare April 26, 2026 19:50

@izambe izambe left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good

@gemini-cli

gemini-cli Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding.

@gemini-cli gemini-cli Bot closed this May 14, 2026
…PATH fallback

After this PR turned `resolveExecutable` into a synchronous function, a
new caller introduced in main (google-gemini#26868) — `await resolveExecutable('rg')`
inside `resolveRipgrepPath` — became an `await` on a non-Promise value
once main was merged into this branch. That triggers ESLint's
`@typescript-eslint/await-thenable` rule and breaks CI lint.

Remove the now-redundant `await`. `resolveRipgrepPath` itself stays
async because it still has two real `await fileExists(...)` checks on
the bundled candidate paths above this call. Behavior is identical;
this is purely an API-signature alignment.

Also update `ripGrep.test.ts` to use `mockReturnValue` instead of
`mockResolvedValue` for `resolveExecutable`, since the mocked function
is no longer a Promise.
@kaluchi

kaluchi commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review.

The auto-close bot fired again on the PR after you reopened it — same misconfigured policy.

The lint failure from the main-merge is fixed and pushed to the branch (PR head won't show it until reopen). The help wanted label would stop the bot loop.

I resolved the main conflict in favor of the sync resolveExecutable:

a) the alternative has a large transitive blast radius — cascading async through the consumers of getShellConfiguration (4 sync sites in shell-utils itself, plus a React render in SessionSummaryDisplay.tsx);

b) the previous async version was already synchronous in effect — serial for ... of over PATH with sequential await fs.promises.access per entry, no Promise.all;

c) the surrounding execution flow has large synchronous sections — command parsing (parsePowerShellCommandDetails uses spawnSync('powershell.exe', ...)), argument-safety checks, and resolveExecutable itself is called from a synchronous prepare stage in shellExecutionService.

@kaluchi

kaluchi commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

@scidomino — the auto-close bot closed the PR again after your reopen (request-review button isn't available while it's closed). Could you reopen and add the 'help wanted' label?

Lint fix and rationale for the sync conflict are in the comment above.

@scidomino scidomino reopened this May 14, 2026
@scidomino scidomino requested a review from a team as a code owner May 14, 2026 15:49
@gemini-cli gemini-cli Bot added help wanted We will accept PRs from all issues marked as "help wanted". Thanks for your support! area/platform Issues related to Build infra, Release mgmt, Testing, Eval infra, Capacity, Quota mgmt labels May 14, 2026
@kaluchi kaluchi requested a review from scidomino May 14, 2026 21:41
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

69 tests passed successfully on gemini-3-flash-preview.

🧠 Model Steering Guidance

This PR modifies files that affect the model's behavior (prompts, tools, or instructions).

  • ⚠️ Consider adding Evals: No behavioral evaluations (evals/*.eval.ts) were added or updated in this PR. Consider adding a test case to verify the new behavior and prevent regressions.
  • 🚀 Maintainer Reminder: Please ensure that these changes do not regress results on benchmark evals before merging.

This is an automated guidance message triggered by steering logic signatures.

getShellConfiguration() on Windows scans PATH for pwsh.exe via
synchronous fs.accessSync on every call. Calling it from a React
render path, even one that runs only at /quit, is undesirable.

SessionSummaryDisplay only reads `.shell` for escapeShellArg.
getShellConfiguration always returns 'powershell' on Windows
(regardless of which executable resolves) and 'bash' on Unix, so the
symbolic shell type is fully determined by the platform. Replace the
call with `isWindows() ? 'powershell' : 'bash'`.

User-visible behavior is unchanged: UUID quoting, malicious-id
escaping, and the Windows double-quote footer wrap (google-gemini#26669) all
follow the same code paths. Drop the now-unused getShellConfiguration
mock from the test setup.
@scidomino scidomino added this pull request to the merge queue May 18, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 18, 2026
@scidomino scidomino added this pull request to the merge queue May 18, 2026
Merged via the queue into google-gemini:main with commit dc47aaa May 18, 2026
29 checks passed
@sripasg sripasg added the size/l A large sized PR label Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/agent Issues related to Core Agent, Tools, Memory, Sub-Agents, Hooks, Agent Quality area/core Issues related to User Interface, OS Support, Core Functionality area/platform Issues related to Build infra, Release mgmt, Testing, Eval infra, Capacity, Quota mgmt help wanted We will accept PRs from all issues marked as "help wanted". Thanks for your support! priority/p1 Important and should be addressed in the near term. priority/p2 Important but can be addressed in a future release. size/l A large sized PR status/pr-nudge-sent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: Regression - run_shell_command strips double quotes

6 participants